-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Search in workspace folder #204
Conversation
workspaceId: workspace.id, | ||
folderId: entry.id, | ||
}, | ||
"*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking this might have been a mistake but actually a good shout to start with * so that there are documents visible straight away before the user adds a search
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep that's what I was thinking. Still want to test this in code with a workspace with loads of documents to see if it slows down the initial folder search.
case (true, _) => ResourceFetchMode.Basic | ||
case (false, text) => ResourceFetchMode.WithData(text) | ||
case (false, pt) => ResourceFetchMode.WithData(pt.map(_.query)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe pc
instead of pt
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parsed tchip
This is looking great, would be good to do a playground test just to check for any url weirdness on a proper domain but otherwise code is looking good to me |
//Attempt.catchNonFatal { | ||
val parsedQ = Json.parse(q) | ||
// find WorkspaceSearchContextParams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be worth adding a slightly more detailed comment here explaning what's happening - i.e extracting the workspace parameters, then ignoring them in the next bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks for walking me through the PR :)
Woohoo! |
What does this change?
How to test
How can we measure success?
Users will be able to search inside a folder inside a workspace.
Potential future changes
Example
giant.search.in.folder.mov